Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: 3 of 7 files reviewed, 4 unresolved discussions (waiting on @AlonLStarkWare)

crates/papyrus_storage/src/body/events.rs line 377 at r3 (raw file):

    DbCursor<'txn, RO, TransactionIndex, VersionZeroWrapper<TransactionMetadata>, SimpleTable>;

/// interface for updating the events in the storage.

capitalization

crates/papyrus_storage/src/body/events.rs line 382 at r3 (raw file):

    Self: Sized,
{
    /// Appends the events of an entire block to the storage.

Copy this comment:

crates/papyrus_storage/src/body/events.rs line 386 at r3 (raw file):

        self,
        block_number: BlockNumber,
        block_events: Vec<Vec<Event>>,

Receive this by reference and as an array slice and not a vec: &[&[Event]]

In general if you don't need to consume the value it's better to pass it by reference (as long as you don't clone it anywhere)
Also, in general it's better to receive &[T] instead of &Vec as the former can handle the latter yet also handle different stuff like arrays (You'll find that useful in tests for example)

crates/papyrus_storage/src/body/events.rs line 425 at r3 (raw file):

    for (index, transaction_events) in block_events.iter().enumerate() {
        let transaction_index = TransactionIndex(block_number, TransactionOffsetInBlock(index));
        let event_offset = file_handlers.append_events(&transaction_events.clone());

Why clone?

Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on @ShahakShama)

crates/papyrus_storage/src/body/events.rs line 377 at r3 (raw file):

Done.

crates/papyrus_storage/src/body/events.rs line 382 at r3 (raw file):

Done.

crates/papyrus_storage/src/body/events.rs line 386 at r3 (raw file):

Done.

crates/papyrus_storage/src/body/events.rs line 425 at r3 (raw file):

Done

Reviewed 1 of 5 files at r4, 2 of 5 files at r8, 4 of 4 files at r9, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @AlonLStarkWare and @noamsp-starkware)

crates/papyrus_storage/src/body/mod.rs line 89 at r9 (raw file):

    CommonPrefix,
>;
// TODO: remove the dead code attribute.

Name this TODO on you

crates/papyrus_storage/src/body/events.rs line 415 at r9 (raw file):

                let transaction_index =
                    TransactionIndex(block_number, TransactionOffsetInBlock(index));
                let event_offset = self.file_handlers.append_events(transaction_events);

event -> events
Is this called offset regularly? maybe file_offset for clarity

crates/papyrus_storage/src/body/events.rs line 417 at r9 (raw file):

                let event_offset = self.file_handlers.append_events(transaction_events);
                events_table.append(&self.txn, &transaction_index, &event_offset)?;
                for event in transaction_events {

It looks like in main we gather all the contract addresses of all the events of a single tx and then write them all together. Could you do the same here?

crates/papyrus_storage/src/lib.rs line 725 at r9 (raw file):

    // Appends an event to the corresponding file and returns its location.
    fn append_events(&self, events: &[Event]) -> LocationInFile {
        self.clone().event.append(&events.to_vec())

If you need to do that, then it's better the type will be Vec
So the outer type should be &[&Vec]

Reviewable status: 4 of 7 files reviewed, 4 unresolved discussions (waiting on @noamsp-starkware and @ShahakShama)

crates/papyrus_storage/src/lib.rs line 725 at r9 (raw file):

Done.

crates/papyrus_storage/src/body/events.rs line 415 at r9 (raw file):

It is only called once. I can change the name to event_location to match the transaction case (where the return value of file_handlers.append_transactions is named tx_location)

crates/papyrus_storage/src/body/mod.rs line 89 at r9 (raw file):

It's already removed in a later PR but sure